-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ISSUE #42] fix the problem HealthExcutor don't doCheck asynchronously #43
Conversation
@@ -41,8 +41,10 @@ public void update(String type, Long typeId, HealthCheckStatus status, String re | |||
cacheMap.put(type, subMap); | |||
} | |||
CheckResult oldResult = subMap.get(typeId); | |||
String oldDesc = Objects.isNull(oldResult.getResultDesc()) ? "" : oldResult.getResultDesc() + "\n"; | |||
CheckResult result = new CheckResult(status, oldDesc + resultDesc, LocalDateTime.now(), | |||
String oldDesc = Objects.isNull(oldResult.getResultDesc()) || oldResult.getResultDesc().isEmpty() ? "" : oldResult.getResultDesc() + "\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why oldDesc
should have a \n
at the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
newdesc
is added after oldDesc
, just to distinguish
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean like this?
oldDesc
newdesc
By the way, Objects.isNull
is intended for use within Java 8 lambda filtering. Just keep the code simple using whatever != null
will be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oldDesc
newdesc
that's what I mean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then such writting will be better:
String whatever = oldDesc + Constants.NEW_LINE_ENDING + newDesc;
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, using a constant is meant to split oldDesc
and newDesc
strings. So the following is better:
String description = oldDesc + Constants.NEW_LINE_ENDING + resultDesc;
Also, a constant can be referenced by its class name to indicate its usage field, so a HEALTH_
prefix is unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if oldDesc
is an empty string ""
, this add an NEW_LINE_ENDING
is the beginning of the description
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if-else is acceptable~
Besides, I add an integration test for HealthService